Migrate cuda_pathfinder/_dynamic_libs search modules from os.path to pathlib - #2489
Migrate cuda_pathfinder/_dynamic_libs search modules from os.path to pathlib#2489LeSingh1 wants to merge 2 commits into
Conversation
os.path is soft-deprecated in favor of the type-safe pathlib API (NVIDIA#2410). Convert the two filesystem-search modules under _dynamic_libs so path construction, joining, and predicates go through Path/PurePath instead of os.path string manipulation, which makes it explicit which values are paths and which are the plain strings the search cascade accumulates for error reporting. Public compatibility is treated as a hard constraint: every entry point still accepts str, and every function that documents or returns str still returns str. Path is used strictly as the internal representation and converted back with str() at each return, so SearchPlatform, FindResult.abs_path, and the LoadedDL.abs_path that reaches users are all unchanged in type and value. Two sites are deliberately left on the old API rather than converted silently, each with a comment explaining why: - _find_lib_dir_using_anchor keeps os.path.normpath. pathlib has no equivalent because PurePath intentionally does not collapse "..", so converting would change the path reported for an anchor such as a CUDA_PATH containing "..". - _derive_ctk_root_windows keeps ntpath. ntpath.dirname slices its input and preserves the caller's separators, while PureWindowsPath rewrites them to backslashes. derive_ctk_root also reaches this function on Linux, so converting would return an unusable root for a POSIX path whose parent directory is named "bin". glob.glob is also kept where wildcard expansion is needed; Path.glob differs in result ordering, and the reverse-sorted newest-first policy from NVIDIA#1732 depends on the current string sort. No behavior change is intended. The only string-level difference is that redundant "." components in an input directory are now collapsed, which cannot occur for the values these functions actually receive. Signed-off-by: LeSingh1 <sshaurya914@gmail.com>
github-actions
Bot
added
the
cuda.pathfinder
|
@mdboom — this is the part 1 I offered on #2410, sent as a small sample so the conversion style and the compatibility approach can be checked before the remaining six parts follow the same template. Happy to hold the rest until you've had a look, or to drop this entirely if you'd rather assign it elsewhere. Two notes for whoever picks it up:
On verification: I don't have CUDA hardware, so I ran the suite on Linux CI rather than locally. Rather than just matching the pass count, I diffed the full pytest output against |
|
A few implementation comments:
This only needs to apply to functions in the public API. We are free to change the implementation details of internal function calls as we see fit, so we should prefer
All uses of |
Follow-up to review feedback on NVIDIA#2489. Narrow the str-compatibility constraint to the public API. The internal helpers and the SearchPlatform protocol now pass and return Path; str() is applied only where the value leaves this module: - FindResult.abs_path, which reaches users unchanged as LoadedDL.abs_path - derive_ctk_root(), whose result is consumed by load_nvidia_dynamic_lib and the header search Replace all four glob.glob() call sites with Path.glob(). Sorting is done on the string form via a small sorted_glob() helper: PurePath ordering is case-insensitive on Windows, so plain sorted() over Path objects would reorder mixed-case filenames relative to the byte-wise ordering these call sites have always used, and the newest-first policy from NVIDIA#1732 rides on that ordering. Two sites deliberately stay on os.path: - _find_lib_dir_using_anchor() keeps os.path.normpath(): PurePath does not collapse "..", and Path.resolve() would additionally follow symlinks. - _derive_ctk_root_windows() keeps ntpath. derive_ctk_root() also reaches it on Linux, where PureWindowsPath would rewrite "/opt/foo/bin/libx.so" to "\opt\foo" instead of "/opt/foo". Differential fuzzing against the previous revision (400k randomized path shapes, 600 randomized directory trees) shows one intended change: LinuxSearchPlatform.conda_anchor_point() now returns a normalized Path rather than $CONDA_PREFIX verbatim. That is not observable downstream, because _find_lib_dir_using_anchor() already normpath'd its result; resolved paths, error messages and listdir attachments are byte-identical. Signed-off-by: LeSingh1 <sshaurya914@gmail.com>
|
Done in 5d5f21a. Internals and the One thing worth flagging:
A correction to my earlier comment. I said the CI baseline was "1188 passed and 88 Differential fuzzing against the previous revision (400k randomized path shapes, 600 randomized directory trees) shows one intended difference: |
Follow-up to the review feedback on NVIDIA#2489: the str-compatibility constraint applies only to the public API. The try_* methods and _no_such_file_in_dir now work in Path throughout. str() is applied once, where abs_path is stored on the public LocatedStaticLib and LocatedBitcodeLib. The relative-path constants go from os.path.join(...) to forward-slash literals, matching how site_packages_dirs is already written in the same dicts; Path normalizes the separator on Windows. One behavior change: a CUDA_PATH or CONDA_PREFIX containing redundant separators ("//", "/.") now produces a normalized abs_path, because Path collapses them. Differential fuzzing against the pre-revision code (16k lookups over randomized trees, comparing located paths and full error text) shows no other difference, and none at all when those variables are free of redundant separators. Signed-off-by: LeSingh1 <sshaurya914@gmail.com>
2 participants
Part 1 of the series proposed in #2410:
cuda_pathfinder/cuda/pathfinder/_dynamic_libs/—search_platform.pyandsearch_steps.py. Deliberately small so the conversion style and the compatibility approach can be reviewed before the remaining six parts follow the same template.What changed
Path construction, joining, and filesystem predicates now go through
Path/PurePathinstead ofos.pathstring manipulation.search_platform.pyno longer importsosat all.Compatibility
Treated as a hard constraint, per the discussion on the issue. Every entry point still accepts
str, and every function that documents or returnsstrstill returnsstr—Pathis used strictly as the internal representation and converted back withstr()at each return.SearchPlatform,FindResult.abs_path, and theLoadedDL.abs_paththat reaches users are unchanged in both type and value. No signature changes.Deliberately not converted
Rather than folding these in silently, both are left alone with a comment saying why:
_find_lib_dir_using_anchorkeepsos.path.normpath. pathlib has no equivalent becausePurePathintentionally does not collapse.., so converting would change the path reported for an anchor such as aCUDA_PATHcontaining..._derive_ctk_root_windowskeepsntpath.ntpath.dirnameslices its input and preserves the caller's separators;PureWindowsPathrewrites them to backslashes.derive_ctk_rootalso reaches this function on Linux, so converting would return an unusable root (\opt\foo) for a POSIX path whose parent directory happens to be namedbin.glob.globis also retained where wildcard expansion is needed —Path.globorders results differently, and the reverse-sorted newest-first policy from #1732 depends on the current string sort.Happy to revisit any of these if you'd prefer them handled differently; that decision will set the pattern for the rest of the series.
Verification
No behavior change intended. Beyond the test suite, I fuzzed the old and new implementations against each other: 200k randomized path shapes for the
derive_ctk_roothelpers, and 3000 randomized real directory trees (including trailing-slash,//and/./spellings) for the search helpers. The only string-level divergence is that a redundant.component in an input directory is now collapsed — the new string is the normalized form of the old and names the same file — and that input cannot occur, sincelib_diralways arrives alreadynormpath-ed.pre-commit runpasses every hook at the pinned versions;mypyis clean overcuda_pathfinder/cuda.Since I don't have CUDA hardware, I verified the suite on Linux CI rather than locally. Against
upstream/mainthe full pytest output is byte-identical apart from elapsed time — same 1188 passed, 4 skipped, and the same 88Not founderrors for absent NVIDIA libraries, in the same order.